-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect non-verbose flags being used in F# scripts or YML CI files #91
Detect non-verbose flags being used in F# scripts or YML CI files #91
Conversation
da363a9
to
daf36a7
Compare
Please rebase this PR 🙏 Thanks |
2d5c202
to
7e9f86f
Compare
Done |
d1bd1c2
to
908c11d
Compare
8e3f57a
to
021111d
Compare
assert | ||
validExtensions | ||
|> Seq.map(fun ext -> fileInfo.FullName.EndsWith(ext)) | ||
|> Seq.contains true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@realmarv can you explain what exactly are you doing here please? this is a bit unreadable; also, why are you using assert instead of failwith? I'm actually not familiar with assert
func TBH, but I have the feeling that it only works in DEBUG mode and the error thrown would be very unreadable (as opposed to using failwith where you are required to provide a readable error)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@realmarv BTW I just cooked this in case you want to use it: nblockchain/fsx@7acb7a5 (will be available in nuget in 5min)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It loops over validExtensions sequence and checks if the file path ends with any of them
@realmarv I raised 2 nits before this can be merged; but please before addressing them, please pull, because I added a commit to your branch |
3e07e1d
to
765a2c4
Compare
@realmarv also PR's CI is red |
Add tests for NonVerboseFlagsInGitHubCI function.
Implement NonVerboseFlagsInGitHubCI function.
Fix NonVerboseFlagsInGitHubCI function.
Cover .sh and .fs files in NonVerboseFlagsInGitHubCI.
Detect non-verbose flags in .yml, .sh, .fs and .fsx files.
Run nonVerboseFlagsInGitHubCIAndFsharpScripts.fsx script and convert non-verbose flags to verbose flags.
199cf64
to
4156fa4
Compare
|> Seq.map(fun ext -> | ||
Helpers.GetInvalidFiles | ||
rootDir | ||
("*" + ext) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@realmarv aren't you missing a dot after * here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No ext already has a dot
4156fa4
to
be256d4
Compare
CI is still red |
It's green now |
No description provided.